Skip to content

Lambdamart implementation#52

Closed
srinikrish22 wants to merge 56 commits intokiudee:masterfrom
srinikrish22:lambdamart-implementation
Closed

Lambdamart implementation#52
srinikrish22 wants to merge 56 commits intokiudee:masterfrom
srinikrish22:lambdamart-implementation

Conversation

@srinikrish22
Copy link
Copy Markdown

I have implemented the LambdaMART algorithm for object ranking.

Description

I have created a class with the required core functionality of lambdamart and added ndcg metric implementation to the utils. I have added the ranker into the test_ranking and also added the ranker definition in the constants.

Motivation and Context

This change aims at adding an implementation of a new ranker for object ranking to the repository.

How Has This Been Tested?

I have added the possibility to test this ranker under the existing testing infrastructure of the cs-ranking repository.

Does this close/impact existing issues?

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.

@kiudee kiudee added the enhancement New feature or request label Aug 26, 2019
@coveralls
Copy link
Copy Markdown

coveralls commented Aug 26, 2019

Pull Request Test Coverage Report for Build 629

  • 165 of 464 (35.56%) changed or added relevant lines in 44 files are covered.
  • 1490 unchanged lines in 45 files lost coverage.
  • Overall coverage decreased (-22.2%) to 31.561%

Changes Missing Coverage Covered Lines Changed/Added Lines %
csrank/choicefunction/fate_choice.py 0 1 0.0%
csrank/choicefunction/fatelinear_choice.py 0 1 0.0%
csrank/choicefunction/fetalinear_choice.py 0 1 0.0%
csrank/choicefunction/generalized_linear_model.py 1 2 50.0%
csrank/choicefunction/pairwise_choice.py 1 2 50.0%
csrank/choicefunction/ranknet_choice.py 0 1 0.0%
csrank/dataset_reader/discretechoice/letor_listwise_discrete_choice_dataset_reader.py 1 2 50.0%
csrank/dataset_reader/labelranking/survey_dataset_reader.py 2 3 66.67%
csrank/dataset_reader/objectranking/depth_dataset_reader.py 0 1 0.0%
csrank/learner.py 0 1 0.0%
Files with Coverage Reduction New Missed Lines %
csrank/dataset_reader/expedia_dataset_reader.py 1 18.66%
csrank/dataset_reader/letor_listwise_dataset_reader.py 1 13.0%
csrank/dataset_reader/letor_ranking_dataset_reader.py 1 13.22%
csrank/dataset_reader/util.py 1 17.7%
csrank/dataset_reader/discretechoice/util.py 2 12.07%
csrank/objectranking/object_ranker.py 3 46.67%
csrank/discretechoice/discrete_choice.py 5 41.18%
csrank/numpy_util.py 6 57.89%
csrank/objectranking/fate_object_ranker.py 6 73.33%
csrank/layers.py 7 68.57%
Totals Coverage Status
Change from base Build 437: -22.2%
Covered Lines: 2481
Relevant Lines: 7861

💛 - Coveralls

Comment thread csrank/metrics.py Outdated
Comment thread csrank/metrics.py Outdated
Comment thread csrank/objectranking/lambdamart.py Outdated
Comment thread csrank/objectranking/lambdamart.py Outdated
Comment thread csrank/objectranking/lambdamart.py Outdated
Comment thread csrank/objectranking/lambdamart.py Outdated
Comment thread csrank/objectranking/lambdamart.py Outdated
Comment thread csrank/objectranking/lambdamart.py Outdated
Comment thread csrank/objectranking/lambdamart.py Outdated
Comment thread csrank/objectranking/lambdamart.py Outdated
timokau and others added 24 commits October 9, 2019 17:46
Keras 2.3 renamed `lr` to `learning_rate`. It is not entirely clear what
convention `tf.keras` will follow (cf.
keras-team/keras#13393), therefore we will
deal with this when we transition to tf2 and tf.keras.
After changing the dependency to Tensorflow<2.0 the ranking tests were
failing, due to small variations in loss values.
To make the tests more robust, we require them to converge to 0 loss now
and thus made the test learning problem much simpler to learn.

This is an overview of the changes:

 * Pin Tensorflow to version 1.x
 * Improve convergence and speed of ranking tests
   - Reduce the number of instances of the learning problem
   - Require learners to reach exactly 0 loss.
Save State
Renamed the package to choice functions

Added a class for standardizing the features

Removed the bug in predicting the scores using pairs for feta discrete choice

Reformatted the code
 -Added the ignore folder in gitignore
 - Added another baseline for Object Ranking problem
We need to escape backslashes in latex code, since python otherwise
interprets \c as an escaped unicode character.

Fixes kiudee#59
Apparently tf.keras now uses[1] `learning_rate` instead of `lr` too, so
we should switch.

[1]keras-team/keras#13393
We do not have control about any of these warnings except the last one.
And that one is harmless in a testing context.

Maintaining a list of these filters is cumbersome. I wish there was some
way to automatically ignore all warnings caused by third party code, but
unfortunately I don't think there currently is. I've opened
pytest-dev/pytest#6191
to discuss this.

I still think it is worthwhile to filter these warnings, because
otherwise they drown out those warnings that are actually relevant.
… metric calculation related implementation. Also added the ranker to the tests
@kiudee
Copy link
Copy Markdown
Owner

kiudee commented Jan 15, 2020

Thank you for the new work on the implementation. Could you rebase your branch onto master and force push it here? Then it would be easier to review the changes.

@prithagupta
Copy link
Copy Markdown
Collaborator

prithagupta commented Jan 15, 2020

I would agree with @kiudee.
@srinikrish22 Secondly, regarding the tests, I would suggest that you follow the pull request #80
run the test locally to just check the environment problem.
The current error is due coverage, which should not have any effect on your implementation work

@srinikrish22
Copy link
Copy Markdown
Author

I have finished the rebase @kiudee. I had done it with my master but forgot to rebase my branch on my master. Now it is be done.
@prithagupta I tried to run the tests locally but my tensorflow installation is broken. I am currently trying to get the tests running on a colab instance.

@prithagupta
Copy link
Copy Markdown
Collaborator

prithagupta commented Jan 17, 2020

@srinikrish22 Did you update the tensorflow, the 2.0 version should have a lot of problems. as far as i know, your ranker does not use tensorflow. So you should just fix your version of the last stable release of the tensorflow.

@kiudee
Copy link
Copy Markdown
Owner

kiudee commented Jan 20, 2020

@srinikrish22 I think you need to first pull the latest master before you do the rebase. Currently, there are still commits in this branch which do not belong to your changes (example 6c4c30c)

@srinikrish22
Copy link
Copy Markdown
Author

srinikrish22 commented Jan 21, 2020

@kiudee I did rebase on the latest master branch recently. Those changes are because of the merge conflicts from the rebase. The last comment on the latest master is 229d5dd on the 19th of November. All my commits then continue after this change.

@srinikrish22
Copy link
Copy Markdown
Author

@kiudee I have created a new pull request to clarify the confusion about rebase with the current master. This should make things easier to compare the changes.

@timokau
Copy link
Copy Markdown
Collaborator

timokau commented Mar 26, 2020

Closing this in favor of #82.

@timokau timokau closed this Mar 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants